-
Notifications
You must be signed in to change notification settings - Fork 503
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
smartlink to fallback on en-US #3133
smartlink to fallback on en-US #3133
Conversation
@chrisdavidmills I basically just took your suggestion. @schalkneethling Please review my use of the @fiji-flo Would it be better to go through the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On my phone, so unable to test. Just a tiny nit for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it, looks and works great, thanks @peterbe!
@schalkneethling I know you have a lot on your plate, but would you mind giving this a quick review for any a11y concerns, specifically regarding the use of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to be so awesome. Thank you, Peter. Just one change recommendation.
kumascript/src/api/web.js
Outdated
return ( | ||
'<a class="page-only-in-english" ' + | ||
'title="Page currently only available in English" ' + | ||
`href="${enUSPage.url}"${flawAttribute}>${content} <small>(en-US)</small></a>` | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return ( | |
'<a class="page-only-in-english" ' + | |
'title="Page currently only available in English" ' + | |
`href="${enUSPage.url}"${flawAttribute}>${content} <small>(en-US)</small></a>` | |
); | |
} | |
return ( | |
'<a class="page-only-in-english" ' + | |
'title="Page currently only available in English" ' + | |
`href="${enUSPage.url}"${flawAttribute}>${content} <span>(en-US)</span></a>` | |
); |
Then in client/src/document/index.scss
add the following:
.page-only-in-english {
span {
font-size: $small-font-size;
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. Will do.
But why add more CSS that can be expressed semantically with HTML? I'm not questioning, I'm trying to learn.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know, sometimes I want to change my mind about the use of these elements but, we would need CSS anyhow to ensure that the small
element uses the $small-font-size
. I am kinda undecided on that one.
e5e04f5
to
968d9cd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 - Thank you @peterbe
* smartLink to fallback on en-US Fixes mdn#3053 * finish * feedbacked
Fixes #3053
Before (http://localhost:3000/ca/docs/Web/JavaScript/Reference/Global_Objects/Array/reverse):
After:
Before (http://localhost:3000/de/docs/Web/CSS/Reference):
After: